Skip to content

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

surechen and others added 16 commits November 12, 2024 10:17
the internal representation of `std::sync::Mutex` depends on the compilation target. due to this,
the compiler produces different number of errors for UI test
`issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected"
error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal
representation regardless of the compilation target. with that change, rustc reports two errors for
all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple
targets.
This fixes a problem where code generated by an external macro with an
RPIT would end up using the call-site edition instead of the macro's
edition for the RPIT. When used from a 2024 crate, this caused the code
to change behavior to the 2024 capturing rules, which we don't want.

This was caused by the impl-trait lowering code would replace the span
with one marked with `DesugaringKind::OpaqueTy` desugaring. However, it
was also overriding the edition of the span with the edition of the
local crate. Instead it should be using the edition of the span itself.

Fixes rust-lang#132917
For expr `return (_ = 42);` unused_paren lint should not be triggered

fixes rust-lang#131989
…ochenkov

Add visit_coroutine_kind to ast::Visitor

r? ``@petrochenkov``

related to rust-lang#128974
…kind, r=compiler-errors

Mention both release *and* edition breakage for never type lints

This PR makes ~~two changes~~ a change to the never type lints (`dependency_on_unit_never_type_fallback` and `never_type_fallback_flowing_into_unsafe`):
1.  Change the wording of the note to mention that the breaking change will be made in an edition _and_ in a future release
2. ~~Make these warnings be reported in deps (hopefully the lints are matured enough)~~

r? ``@compiler-errors``
cc ``@ehuss``
closes rust-lang#132930
…tic, r=Noratrieb

make UI test OS-agnostic

the internal representation of `std::sync::Mutex` depends on the compilation target. due to this, the compiler produces different number of errors for UI test `issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected" error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal representation regardless of the compilation target. with that change, rustc reports two errors for all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple targets.
…iler-errors

Fix span edition for 2024 RPIT coming from an external macro

This fixes a problem where code generated by an external macro with an RPIT would end up using the call-site edition instead of the macro's edition for the RPIT. When used from a 2024 crate, this caused the code to change behavior to the 2024 capturing rules, which we don't want.

This was caused by the impl-trait lowering code would replace the span with one marked with `DesugaringKind::OpaqueTy` desugaring. However, it was also overriding the edition of the span with the edition of the local crate. Instead it should be using the edition of the span itself.

Fixes rust-lang#132917
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Nov 15, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ p= rollup=never

@bors
Copy link
Collaborator

bors commented Nov 15, 2024

📌 Commit fc8d2b3 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2024
@bors
Copy link
Collaborator

bors commented Nov 15, 2024

⌛ Testing commit fc8d2b3 with merge d3a4b1f...

@bors
Copy link
Collaborator

bors commented Nov 16, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing d3a4b1f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 16, 2024
@bors bors merged commit d3a4b1f into rust-lang:master Nov 16, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 16, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#132936 For expr return (_ = 42); unused_paren lint should not be… cd03af90f32af8013e44b83f3ba34850b6bd5189 (link)
#132956 Add visit_coroutine_kind to ast::Visitor 5892f875b6ee3e9adf25f2a1b431dc0faa764bd5 (link)
#132978 Mention both release and edition breakage for never type … a58dc12e1e4c013d5e82d21376b58601ed45448a (link)
#133074 make UI test OS-agnostic a2a7de7fff0158fe584a5c44c9b882770592304d (link)
#133080 Fix span edition for 2024 RPIT coming from an external macro 943783eeffd58810ae2aa4cf3b2f58ad3d1424a6 (link)

previous master: 917a50a039

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d3a4b1f): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.2% [-4.2%, -4.2%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 2.1%, secondary 1.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
2.6% [2.0%, 3.5%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.8% [-4.8%, -4.8%] 1
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Cycles

Results (secondary 19.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
19.2% [19.2%, 19.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 788.599s -> 791.497s (0.37%)
Artifact size: 335.45 MiB -> 335.43 MiB (-0.00%)

@GuillaumeGomez GuillaumeGomez deleted the rollup-kbkfrkj branch November 16, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants